Fix Concatenate and Generic with ParamSpec substitution #489
Conversation
|
The code could be slimmed down in some cases if we introduce |
Not that I'm aware of. But I think functions in typing.py and elsewhere don't pay attention to these hooks on Python less than 3.11(?), so it might be less effective to add them than you might think. (I'm not confident in this opinion, definitely try it out if you think it might work!) |
- nested concatenate - Unpack subscription
Daraan
force-pushed
the
ParamSpec/Generic
branch
from
83dbe19
to
ee810d9
Compare
| @@ -3171,6 +3309,13 @@ def _collect_type_vars(types, typevar_types=None): | |||
| tvars.append(t) | |||
| if _should_collect_from_parameters(t): | |||
| tvars.extend([t for t in t.__parameters__ if t not in tvars]) | |||
| elif isinstance(t, tuple): | |||
There was a problem hiding this comment.
This is a copied up from _collect_paramters needed to detect the parameters in the cases G5, G6, H9, G10
src/typing_extensions.py
Outdated
| if (inspect.isclass(cls) and issubclass(cls, typing.Generic) | ||
| and any(isinstance(t, ParamSpec) for t in cls.__parameters__) | ||
| ): | ||
| # should actually modify parameters but is immutable | ||
| if ( | ||
| len(cls.__parameters__) == 1 | ||
| and parameters | ||
| and not _is_param_expr(parameters[0]) | ||
| ): | ||
| assert isinstance(cls.__parameters__[0], ParamSpec) | ||
| return |
There was a problem hiding this comment.
See PR #491 for this base.
Note changes here are the combined checks from _prepare_paramspec_params(cls, params) and Generic.__class_getitem__ that would normally not execute _check_generic in 3.10+
| def _unpack_args(*args): | ||
| newargs = [] |
There was a problem hiding this comment.
Needed for 3.8-3.10 to unpack Unpack during substitution, e.g.:
C2 = Concatenate[str, P]
U1 = Unpack[Tuple[int, str]]
self.assertEqual(C2[U1], (str, int, str))|
|
||
| # This is an invalid parameter expression but useful for testing correct subsitution | ||
| G10 = klass[int, Concatenate[str, P]] | ||
| with self.subTest("Check invalid form substitution"): |
There was a problem hiding this comment.
This is not a valid parameter expression, it is nice for debugging though, should keep or remove it?
src/typing_extensions.py
Outdated
| return None | ||
|
|
||
| def __getattr__(self, attr): | ||
| if attr == '__typing_unpacked_tuple_args__': |
There was a problem hiding this comment.
Why does this need to go through __getattr__? Why can't we add a property named __typing_unpacked_tuple_args__?
There was a problem hiding this comment.
I had a reason once, but I do not see it anymore. Is now a property.
src/typing_extensions.py
Outdated
| @@ -3007,6 +3136,12 @@ def wrapper(*args, **kwargs): | |||
| ) | |||
|
|
|||
|
|
|||
| def _is_param_expr(arg): | |||
| return arg is ... or isinstance( | |||
| arg, (tuple, list, ParamSpec, _ConcatenateGenericAlias) | |||
There was a problem hiding this comment.
Do we need to cover the typing as well as the typing-extensions versions of the last two?
There was a problem hiding this comment.
For _ConcatenateGenericAlias we indeed need it. Fixed and added also tests for both.
this PR is is a follow up on #491 and addresses multiple locations that case problems for different situations and versions; see comments below.